[SFrames] Emit and relax FREs #158154
Merged
[SFrames] Emit and relax FREs #158154
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
After this change, llvm emits usable sframe sections that can be compared to the gnu-gas implementation and linked with the gnu linker. There are a few remaining cfi directives to handle before we have complete compatibility, however. The output isn't identical with gnu in every case (this code produces fewer identical FREs in a row than gas), but I'm reasonably sure that they are correct. There are even more size optimizations that can be done later. This is a fairly big commit, but I'm not sure how to make it smaller. Also, while working on the tests, I found a few bugs in older portions and cleaned those up.
|
@llvm/pr-subscribers-llvm-binary-utilities @llvm/pr-subscribers-llvm-mc Author: None (Sterling-Augustine) ChangesThis PR emits and relaxes the FREs generated in the previous PR. After this change llvm emits usable sframe sections that can be The output isn't identical with gnu-gas in every case (this code produces Also, while working on the tests, I found a few bugs in older portions This is a fairly big commit, but I'm not sure how to make it smaller. Patch is 30.60 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/158154.diff 13 Files Affected:
diff --git a/llvm/include/llvm/BinaryFormat/SFrame.h b/llvm/include/llvm/BinaryFormat/SFrame.h
index 095db18b9c254..7b58043c60363 100644
--- a/llvm/include/llvm/BinaryFormat/SFrame.h
+++ b/llvm/include/llvm/BinaryFormat/SFrame.h
@@ -117,6 +117,7 @@ template <endianness E> struct FDEInfo {
Info = ((PAuthKey & 1) << 5) | ((static_cast<uint8_t>(FDE) & 1) << 4) |
(static_cast<uint8_t>(FRE) & 0xf);
}
+ uint8_t getFuncInfo() const { return Info; }
};
template <endianness E> struct FuncDescEntry {
@@ -155,6 +156,7 @@ template <endianness E> struct FREInfo {
Info = ((RA & 1) << 7) | ((static_cast<uint8_t>(Sz) & 3) << 5) |
((N & 0xf) << 1) | (static_cast<uint8_t>(Reg) & 1);
}
+ uint8_t getFREInfo() const { return Info; }
};
template <typename T, endianness E> struct FrameRowEntry {
diff --git a/llvm/include/llvm/MC/MCAsmBackend.h b/llvm/include/llvm/MC/MCAsmBackend.h
index 1625355323692..58363f0b671e2 100644
--- a/llvm/include/llvm/MC/MCAsmBackend.h
+++ b/llvm/include/llvm/MC/MCAsmBackend.h
@@ -168,6 +168,7 @@ class LLVM_ABI MCAsmBackend {
virtual bool relaxAlign(MCFragment &F, unsigned &Size) { return false; }
virtual bool relaxDwarfLineAddr(MCFragment &) const { return false; }
virtual bool relaxDwarfCFA(MCFragment &) const { return false; }
+ virtual bool relaxSFrameCFA(MCFragment &) const { return false; }
// Defined by linker relaxation targets to possibly emit LEB128 relocations
// and set Value at the relocated location.
diff --git a/llvm/include/llvm/MC/MCAssembler.h b/llvm/include/llvm/MC/MCAssembler.h
index 1316d8669239d..6e1d6421b8d33 100644
--- a/llvm/include/llvm/MC/MCAssembler.h
+++ b/llvm/include/llvm/MC/MCAssembler.h
@@ -117,6 +117,7 @@ class MCAssembler {
void relaxBoundaryAlign(MCBoundaryAlignFragment &BF);
void relaxDwarfLineAddr(MCFragment &F);
void relaxDwarfCallFrameFragment(MCFragment &F);
+ void relaxSFrameFragment(MCFragment &DF);
public:
/// Construct a new assembler instance.
diff --git a/llvm/include/llvm/MC/MCObjectStreamer.h b/llvm/include/llvm/MC/MCObjectStreamer.h
index b9e813b9b0d28..1899cb6331c6f 100644
--- a/llvm/include/llvm/MC/MCObjectStreamer.h
+++ b/llvm/include/llvm/MC/MCObjectStreamer.h
@@ -150,6 +150,9 @@ class MCObjectStreamer : public MCStreamer {
MCSymbol *EndLabel = nullptr) override;
void emitDwarfAdvanceFrameAddr(const MCSymbol *LastLabel,
const MCSymbol *Label, SMLoc Loc);
+ void emitSFrameCalculateFuncOffset(const MCSymbol *FunCabsel,
+ const MCSymbol *FREBegin,
+ MCFragment *FDEFrag, SMLoc Loc);
void emitCVLocDirective(unsigned FunctionId, unsigned FileNo, unsigned Line,
unsigned Column, bool PrologueEnd, bool IsStmt,
StringRef FileName, SMLoc Loc) override;
diff --git a/llvm/include/llvm/MC/MCSFrame.h b/llvm/include/llvm/MC/MCSFrame.h
index 8f182a86d1ab1..694aec55aefeb 100644
--- a/llvm/include/llvm/MC/MCSFrame.h
+++ b/llvm/include/llvm/MC/MCSFrame.h
@@ -16,9 +16,14 @@
#ifndef LLVM_MC_MCSFRAME_H
#define LLVM_MC_MCSFRAME_H
+#include "llvm/ADT/SmallVector.h"
+#include <cstdint>
+
namespace llvm {
+class MCContext;
class MCObjectStreamer;
+class MCFragment;
class MCSFrameEmitter {
public:
@@ -26,6 +31,15 @@ class MCSFrameEmitter {
//
// \param Streamer - Emit into this stream.
static void emit(MCObjectStreamer &Streamer);
+
+ // Encode the FRE's function offset.
+ //
+ // \param C - Context.
+ // \param Offset - Offset to encode.
+ // \param Out - Destination of the encoding.
+ // \param FDEFrag - Frag that specifies the encoding format.
+ static void encodeFuncOffset(MCContext &C, uint64_t Offset,
+ SmallVectorImpl<char> &Out, MCFragment *FDEFrag);
};
} // end namespace llvm
diff --git a/llvm/include/llvm/MC/MCSection.h b/llvm/include/llvm/MC/MCSection.h
index 12389d623e588..a26e6cfb2158a 100644
--- a/llvm/include/llvm/MC/MCSection.h
+++ b/llvm/include/llvm/MC/MCSection.h
@@ -59,6 +59,7 @@ class MCFragment {
FT_Org,
FT_Dwarf,
FT_DwarfFrame,
+ FT_SFrame,
FT_BoundaryAlign,
FT_SymbolId,
FT_CVInlineLines,
@@ -143,6 +144,12 @@ class MCFragment {
// .loc dwarf directives.
int64_t LineDelta;
} dwarf;
+ struct {
+ // This FRE describes unwind info at AddrDelta from function start.
+ const MCExpr *AddrDelta;
+ // Fragment that records how many bytes of AddrDelta to emit.
+ MCFragment *FDEFragment;
+ } sframe;
} u{};
public:
@@ -296,6 +303,24 @@ class MCFragment {
assert(Kind == FT_Dwarf);
u.dwarf.LineDelta = LineDelta;
}
+
+ //== FT_SFrame functions
+ const MCExpr &getSFrameAddrDelta() const {
+ assert(Kind == FT_SFrame);
+ return *u.sframe.AddrDelta;
+ }
+ void setSFrameAddrDelta(const MCExpr *E) {
+ assert(Kind == FT_SFrame);
+ u.sframe.AddrDelta = E;
+ }
+ MCFragment *getSFrameFDE() const {
+ assert(Kind == FT_SFrame);
+ return u.sframe.FDEFragment;
+ }
+ void setSFrameFDE(MCFragment *F) {
+ assert(Kind == FT_SFrame);
+ u.sframe.FDEFragment = F;
+ }
};
// MCFragment subclasses do not use the fixed-size part or variable-size tail of
diff --git a/llvm/lib/MC/MCAssembler.cpp b/llvm/lib/MC/MCAssembler.cpp
index b1031d7822604..cee281597cfed 100644
--- a/llvm/lib/MC/MCAssembler.cpp
+++ b/llvm/lib/MC/MCAssembler.cpp
@@ -22,6 +22,7 @@
#include "llvm/MC/MCFixup.h"
#include "llvm/MC/MCInst.h"
#include "llvm/MC/MCObjectWriter.h"
+#include "llvm/MC/MCSFrame.h"
#include "llvm/MC/MCSection.h"
#include "llvm/MC/MCSymbol.h"
#include "llvm/MC/MCValue.h"
@@ -199,6 +200,7 @@ uint64_t MCAssembler::computeFragmentSize(const MCFragment &F) const {
case MCFragment::FT_LEB:
case MCFragment::FT_Dwarf:
case MCFragment::FT_DwarfFrame:
+ case MCFragment::FT_SFrame:
case MCFragment::FT_CVInlineLines:
case MCFragment::FT_CVDefRange:
return F.getSize();
@@ -399,6 +401,7 @@ static void writeFragment(raw_ostream &OS, const MCAssembler &Asm,
case MCFragment::FT_LEB:
case MCFragment::FT_Dwarf:
case MCFragment::FT_DwarfFrame:
+ case MCFragment::FT_SFrame:
case MCFragment::FT_CVInlineLines:
case MCFragment::FT_CVDefRange: {
if (F.getKind() == MCFragment::FT_Data)
@@ -914,6 +917,24 @@ void MCAssembler::relaxDwarfCallFrameFragment(MCFragment &F) {
F.clearVarFixups();
}
+void MCAssembler::relaxSFrameFragment(MCFragment &F) {
+ assert(F.getKind() == MCFragment::FT_SFrame);
+ MCContext &C = getContext();
+ int64_t Value;
+ bool Abs = F.getSFrameAddrDelta().evaluateAsAbsolute(Value, *this);
+ if (!Abs) {
+ C.reportError(F.getSFrameAddrDelta().getLoc(),
+ "invalid CFI advance_loc expression in sframe");
+ F.setSFrameAddrDelta(MCConstantExpr::create(0, C));
+ return;
+ }
+
+ SmallVector<char, 4> Data;
+ MCSFrameEmitter::encodeFuncOffset(Context, Value, Data, F.getSFrameFDE());
+ F.setVarContents(Data);
+ F.clearVarFixups();
+}
+
bool MCAssembler::relaxFragment(MCFragment &F) {
auto Size = computeFragmentSize(F);
switch (F.getKind()) {
@@ -932,6 +953,9 @@ bool MCAssembler::relaxFragment(MCFragment &F) {
case MCFragment::FT_DwarfFrame:
relaxDwarfCallFrameFragment(F);
break;
+ case MCFragment::FT_SFrame:
+ relaxSFrameFragment(F);
+ break;
case MCFragment::FT_BoundaryAlign:
relaxBoundaryAlign(static_cast<MCBoundaryAlignFragment &>(F));
break;
diff --git a/llvm/lib/MC/MCFragment.cpp b/llvm/lib/MC/MCFragment.cpp
index 21da79bb0aa30..85d1c5888f1da 100644
--- a/llvm/lib/MC/MCFragment.cpp
+++ b/llvm/lib/MC/MCFragment.cpp
@@ -53,6 +53,7 @@ LLVM_DUMP_METHOD void MCFragment::dump() const {
case MCFragment::FT_Org: OS << "Org"; break;
case MCFragment::FT_Dwarf: OS << "Dwarf"; break;
case MCFragment::FT_DwarfFrame: OS << "DwarfCallFrame"; break;
+ case MCFragment::FT_SFrame: OS << "SFrame"; break;
case MCFragment::FT_LEB: OS << "LEB"; break;
case MCFragment::FT_BoundaryAlign: OS<<"BoundaryAlign"; break;
case MCFragment::FT_SymbolId: OS << "SymbolId"; break;
@@ -79,7 +80,8 @@ LLVM_DUMP_METHOD void MCFragment::dump() const {
case MCFragment::FT_Align:
case MCFragment::FT_LEB:
case MCFragment::FT_Dwarf:
- case MCFragment::FT_DwarfFrame: {
+ case MCFragment::FT_DwarfFrame:
+ case MCFragment::FT_SFrame: {
if (isLinkerRelaxable())
OS << " LinkerRelaxable";
auto Fixed = getContents();
@@ -129,6 +131,7 @@ LLVM_DUMP_METHOD void MCFragment::dump() const {
OS << " LineDelta:" << getDwarfLineDelta();
break;
case MCFragment::FT_DwarfFrame:
+ case MCFragment::FT_SFrame:
OS << " AddrDelta:";
getDwarfAddrDelta().print(OS, nullptr);
break;
diff --git a/llvm/lib/MC/MCObjectStreamer.cpp b/llvm/lib/MC/MCObjectStreamer.cpp
index 59265bc8595ba..701a0836d2c70 100644
--- a/llvm/lib/MC/MCObjectStreamer.cpp
+++ b/llvm/lib/MC/MCObjectStreamer.cpp
@@ -583,6 +583,19 @@ void MCObjectStreamer::emitDwarfAdvanceFrameAddr(const MCSymbol *LastLabel,
newFragment();
}
+void MCObjectStreamer::emitSFrameCalculateFuncOffset(const MCSymbol *FuncBase,
+ const MCSymbol *FREBegin,
+ MCFragment *FDEFrag,
+ SMLoc Loc) {
+ assert(FuncBase && "No function base address");
+ assert(FREBegin && "FRE doesn't describe a location");
+ auto *F = getCurrentFragment();
+ F->Kind = MCFragment::FT_SFrame;
+ F->setSFrameAddrDelta(buildSymbolDiff(*this, FREBegin, FuncBase, Loc));
+ F->setSFrameFDE(FDEFrag);
+ newFragment();
+}
+
void MCObjectStreamer::emitCVLocDirective(unsigned FunctionId, unsigned FileNo,
unsigned Line, unsigned Column,
bool PrologueEnd, bool IsStmt,
diff --git a/llvm/lib/MC/MCSFrame.cpp b/llvm/lib/MC/MCSFrame.cpp
index a0d6c80ab72ea..60446d1657034 100644
--- a/llvm/lib/MC/MCSFrame.cpp
+++ b/llvm/lib/MC/MCSFrame.cpp
@@ -14,6 +14,7 @@
#include "llvm/MC/MCObjectStreamer.h"
#include "llvm/MC/MCSection.h"
#include "llvm/MC/MCSymbol.h"
+#include "llvm/Support/Endian.h"
#include "llvm/Support/EndianStream.h"
using namespace llvm;
@@ -33,10 +34,86 @@ struct SFrameFRE {
size_t CFAOffset = 0;
size_t FPOffset = 0;
size_t RAOffset = 0;
- bool FromFP = false;
+ FREInfo<endianness::native> Info;
bool CFARegSet = false;
SFrameFRE(const MCSymbol *Start) : Label(Start) {}
+
+ void emit(MCObjectStreamer &S, const MCSymbol *FuncBegin,
+ MCFragment *FDEFrag) {
+ S.emitSFrameCalculateFuncOffset(FuncBegin, Label, FDEFrag, SMLoc());
+
+ // fre_cfa_base_reg_id already set during parsing
+
+ // fre_offset_count
+ unsigned RegsTracked = 1; // always track the cfa.
+ if (FPOffset != 0)
+ RegsTracked++;
+ if (RAOffset != 0)
+ RegsTracked++;
+ Info.setOffsetCount(RegsTracked);
+
+ // fre_offset_size
+ if (isInt<8>(CFAOffset) && isInt<8>(FPOffset) && isInt<8>(RAOffset))
+ Info.setOffsetSize(FREOffset::B1);
+ else if (isInt<16>(CFAOffset) && isInt<16>(FPOffset) && isInt<16>(RAOffset))
+ Info.setOffsetSize(FREOffset::B2);
+ else {
+ assert(isInt<32>(CFAOffset) && isInt<32>(FPOffset) &&
+ isInt<32>(RAOffset) && "Offset too big for sframe");
+ Info.setOffsetSize(FREOffset::B4);
+ }
+
+ // No support for fre_mangled_ra_p yet.
+ Info.setReturnAddressSigned(false);
+
+ // sframe_fre_info_word
+ S.emitInt8(Info.getFREInfo());
+
+ // FRE Offsets
+ [[maybe_unused]] unsigned OffsetsEmitted = 1;
+ switch (Info.getOffsetSize()) {
+ case (FREOffset::B1):
+ S.emitInt8(CFAOffset);
+ break;
+ case (FREOffset::B2):
+ S.emitInt16(CFAOffset);
+ break;
+ case (FREOffset::B4):
+ S.emitInt32(CFAOffset);
+ break;
+ }
+ if (FPOffset) {
+ OffsetsEmitted++;
+ switch (Info.getOffsetSize()) {
+ case (FREOffset::B1):
+ S.emitInt8(FPOffset);
+ break;
+ case (FREOffset::B2):
+ S.emitInt16(FPOffset);
+ break;
+ case (FREOffset::B4):
+ S.emitInt32(FPOffset);
+ break;
+ }
+ }
+ if (RAOffset) {
+ OffsetsEmitted++;
+ switch (Info.getOffsetSize()) {
+ case (FREOffset::B1):
+ S.emitInt8(RAOffset);
+ break;
+ case (FREOffset::B2):
+ S.emitInt16(RAOffset);
+ break;
+ case (FREOffset::B4):
+ S.emitInt32(RAOffset);
+ break;
+ }
+ }
+ assert(OffsetsEmitted == RegsTracked &&
+ "Didn't emit the right number of offsets");
+ }
};
// High-level structure to track info needed to emit a sframe_func_desc_entry
@@ -46,11 +123,13 @@ struct SFrameFDE {
const MCDwarfFrameInfo &DFrame;
// Label where this FDE's FREs start.
MCSymbol *FREStart;
+ // Frag where this FDE is emitted.
+ MCFragment *Frag;
// Unwinding fres
SmallVector<SFrameFRE> FREs;
SFrameFDE(const MCDwarfFrameInfo &DF, MCSymbol *FRES)
- : DFrame(DF), FREStart(FRES) {}
+ : DFrame(DF), FREStart(FRES), Frag(nullptr) {}
void emit(MCObjectStreamer &S, const MCSymbol *FRESubSectionStart) {
MCContext &C = S.getContext();
@@ -74,13 +153,21 @@ struct SFrameFDE {
S.emitInt32(0);
// sfde_func_num_fres
- // TODO: When we actually emit fres, replace 0 with FREs.size()
- S.emitInt32(0);
+ S.emitInt32(FREs.size());
// sfde_func_info word
- FDEInfo<endianness::native> I;
- I.setFuncInfo(0 /* No pauth key */, FDEType::PCInc, FREType::Addr1);
- S.emitInt8(I.Info);
+
+ // All FREs within an FDE share the same sframe::FREType::AddrX. The value
+ // of 'X' is determined by the FRE with the largest offset, which is the
+ // last. This offset isn't known until relax time, so emit a frag which can
+ // calculate that now.
+ //
+ // At relax time, this FDE frag calculates the proper AddrX value (as well
+ // as the rest of the FDE FuncInfo word). Subsequent FRE frags will read it
+ // from this frag and emit the proper number of bytes.
+ Frag = S.getCurrentFragment();
+ S.emitSFrameCalculateFuncOffset(DFrame.Begin, FREs.back().Label, nullptr,
+ SMLoc());
// sfde_func_rep_size. Not relevant in non-PCMASK fdes.
S.emitInt8(0);
@@ -96,13 +183,16 @@ struct SFrameFDE {
class SFrameEmitterImpl {
MCObjectStreamer &Streamer;
SmallVector<SFrameFDE> FDEs;
+ uint32_t TotalFREs;
ABI SFrameABI;
// Target-specific convenience variables to detect when a CFI instruction
// references these registers. Unlike in dwarf frame descriptions, they never
- // escape into the sframe section itself.
+ // escape into the sframe section itself. TODO: These should be retrieved from
+ // the target.
unsigned SPReg;
unsigned FPReg;
unsigned RAReg;
+ int8_t FixedRAOffset;
MCSymbol *FDESubSectionStart;
MCSymbol *FRESubSectionStart;
MCSymbol *FRESubSectionEnd;
@@ -110,12 +200,12 @@ class SFrameEmitterImpl {
bool setCFARegister(SFrameFRE &FRE, const MCCFIInstruction &I) {
if (I.getRegister() == SPReg) {
FRE.CFARegSet = true;
- FRE.FromFP = false;
+ FRE.Info.setBaseRegister(BaseReg::SP);
return true;
}
if (I.getRegister() == FPReg) {
FRE.CFARegSet = true;
- FRE.FromFP = true;
+ FRE.Info.setBaseRegister(BaseReg::FP);
return true;
}
Streamer.getContext().reportWarning(
@@ -182,7 +272,8 @@ class SFrameEmitterImpl {
}
public:
- SFrameEmitterImpl(MCObjectStreamer &Streamer) : Streamer(Streamer) {
+ SFrameEmitterImpl(MCObjectStreamer &Streamer)
+ : Streamer(Streamer), TotalFREs(0) {
assert(Streamer.getContext()
.getObjectFileInfo()
->getSFrameABIArch()
@@ -195,6 +286,7 @@ class SFrameEmitterImpl {
SPReg = 31;
RAReg = 29;
FPReg = 30;
+ FixedRAOffset = 0;
break;
case ABI::AMD64EndianLittle:
SPReg = 7;
@@ -202,6 +294,7 @@ class SFrameEmitterImpl {
// MCDwarfFrameInfo constructor.
RAReg = static_cast<unsigned>(INT_MAX);
FPReg = 6;
+ FixedRAOffset = -8;
break;
}
@@ -219,10 +312,16 @@ class SFrameEmitterImpl {
bool equalIgnoringLocation(const SFrameFRE &Left, const SFrameFRE &Right) {
return Left.CFAOffset == Right.CFAOffset &&
Left.FPOffset == Right.FPOffset && Left.RAOffset == Right.RAOffset &&
- Left.FromFP == Right.FromFP && Left.CFARegSet == Right.CFARegSet;
+ Left.Info.getFREInfo() == Right.Info.getFREInfo() &&
+ Left.CFARegSet == Right.CFARegSet;
}
void buildSFDE(const MCDwarfFrameInfo &DF) {
+ // Functions with zero size can happen with assembler macros and
+ // machine-generated code. They don't need unwind info at all, so
+ // no need to warn.
+ if (atSameLocation(DF.Begin, DF.End))
+ return;
bool Valid = true;
SFrameFDE FDE(DF, Streamer.getContext().createTempSymbol());
// This would have been set via ".cfi_return_column", but
@@ -277,8 +376,11 @@ class SFrameEmitterImpl {
LastLabel = L;
}
}
- if (Valid)
+
+ if (Valid) {
FDEs.push_back(FDE);
+ TotalFREs += FDE.FREs.size();
+ }
}
void emitPreamble() {
@@ -294,13 +396,12 @@ class SFrameEmitterImpl {
// sfh_cfa_fixed_fp_offset
Streamer.emitInt8(0);
// sfh_cfa_fixed_ra_offset
- Streamer.emitInt8(0);
+ Streamer.emitInt8(FixedRAOffset);
// sfh_auxhdr_len
Streamer.emitInt8(0);
// shf_num_fdes
Streamer.emitInt32(FDEs.size());
// shf_num_fres
- uint32_t TotalFREs = 0;
Streamer.emitInt32(TotalFREs);
// shf_fre_len
@@ -322,8 +423,11 @@ class SFrameEmitterImpl {
void emitFREs() {
Streamer.emitLabel(FRESubSectionStart);
- for (auto &FDE : FDEs)
+ for (auto &FDE : FDEs) {
Streamer.emitLabel(FDE.FREStart);
+ for (auto &FRE : FDE.FREs)
+ FRE.emit(Streamer, FDE.DFrame.Begin, FDE.Frag);
+ }
Streamer.emitLabel(FRESubSectionEnd);
}
};
@@ -359,3 +463,51 @@ void MCSFrameEmitter::emit(MCObjectStreamer &Streamer) {
Emitter.emitFDEs();
Emitter.emitFREs();
}
+
+void MCSFrameEmitter::encodeFuncOffset(MCContext &C, uint64_t Offset,
+ SmallVectorImpl<char> &Out,
+ MCFragment *FDEFrag) {
+ // If encoding into the FDE Frag itself, generate the sfde_info_word.
+ if (FDEFrag == nullptr) {
+ // sfde_func_info
+
+ // Offset is the difference between the function start label and the final
+ // FRE's offset, which is the max offset for this FDE.
+ FDEInfo<endianness::native> I;
+ if (isUInt<8>(Offset))
+ I.setFREType(FREType::Addr1);
+ else if (isUInt<16>(Offset))
+ I.setFREType(FREType::Addr2);
+ else {
+ assert(isUInt<32>(Offset));
+ I.setFREType(FREType::Addr4);
+ }
+ I.setFDEType(FDEType::PCInc);
+
+ Out.push_back(I.getFuncInfo());
+ return;
+ }
+
+ const auto &FDEData = FDEFrag->getVarContents();
+ FDEInfo<endianness::native> I;
+ I.Info = FDEData.back();
+ FREType T = I.getFREType();
+ llvm::endianness E = C.getAsmInfo()->isLittleEndian()
+ ? llvm::endianness::little
+ : llvm::endianness::big;
+ // sfre_start_address
+ switch (T) {
+ case FREType::Addr1:
+ assert(isUInt<8>(Offset) && "Miscalculated Sframe FREType");
+ support::endian::write<uint8_t>(Out, Offset, E);
+ break;
+ case FREType::Addr2:
+ assert(isUInt<16>(Offset) && "Miscalculated Sframe FREType");
+ support::endian::write<uint16_t>(Out, Offset, E);
+ break;
+ case FREType::Addr4:
+ assert(isUInt<32>(Offset) && "Miscalculated Sframe FREType");
+ support::endian::write<uint32_t>(Out, Offset, E);
+ break;
+ }
+}
diff --git a/llvm/test/MC/ELF/cfi-sframe-encoding.s b/llvm/test/MC/ELF/cfi-sframe-encoding.s
new file mode 100644
index 0000000000000..e13e11c51c05a
--- /dev/null
+++ b/llvm/test/MC/ELF/cfi-sframe-encoding.s
@@ -0,0 +1,87 @@
+// TODO: Add other architectures as they gain sframe support
+// REQUIRES: x86-registered-target
+// RUN: llvm-mc --assemble --file...
[truncated]
|
weiguozhi
reviewed
Sep 12, 2025
weiguozhi
reviewed
Sep 12, 2025
|
OK to commit? |
|
SFrame part looks good to me. It's better to have another review for the MC part. |
|
It's been a week since I requested review. I'll commit it tomorrow morning if no one chimes in by then. |
Sterling-Augustine
added a commit
that referenced
this pull request
Sep 17, 2025
MaskRay
reviewed
Sep 18, 2025
| // fre_offset_count | ||
| unsigned RegsTracked = 1; // always track the cfa. | ||
| if (FPOffset != 0) | ||
| RegsTracked++; |
There was a problem hiding this comment.
LLVM coding style prefers pre-increment
| @@ -0,0 +1,87 @@ | |||
| // TODO: Add other architectures as they gain sframe support | |||
| // REQUIRES: x86-registered-target | |||
There was a problem hiding this comment.
MC/ELF/lit.local.cfg tests X86. // REQUIRES: x86-registered-target is not needed
Delete this TODO. Support for other architectures generally go to test/MC/$arch/
In newer tests, # is more popular as RUN/CHECK prefixes and ## is for regular comments.
llvm-mc doesn't need --assemble
| newFragment(); | ||
| } | ||
|
|
||
| void MCObjectStreamer::emitSFrameCalculateFuncOffset(const MCSymbol *FuncBase, |
There was a problem hiding this comment.
Prefer const reference to non-nullable pointer
There was a problem hiding this comment.
Every single MCObjectStreamer::emit* that takes an MCSymbol takes it by pointer, as does every emit* function in the superclass MCStreamer. And all the other subclasses. I think I should leave this as is to match the surrounding code. Hopefully a broad cleanup will migrate everything.
|
This pr had previously been submitted, but was reverted due to two uninitialized padding bits making it fail msan testing. Relanding shortly, but with these new comments addressed. |
Sterling-Augustine
added a commit
that referenced
this pull request
Sep 18, 2025
[Previously reverted due to msan failures on two uninitialized padding bits.] This PR emits and relaxes the FREs generated in the previous PR. After this change llvm emits usable sframe sections that can be linked with the gnu linker. There are a few remaining cfi directives to handle before they are generally usable, however. The output isn't identical with gnu-gas in every case (this code produces fewer identical FREs in a row than gas), but I'm reasonably sure that they are correct regardless. There are even more size optimizations that can be done later. Also, while working on the tests, I found a few bugs in older portions and cleaned those up. This is a fairly big commit, but I'm not sure how to make it smaller.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR emits and relaxes the FREs generated in the previous PR.
After this change llvm emits usable sframe sections that can be
linked with the gnu linker. There are a few remaining cfi directives to
handle before they are generally usable, however.
The output isn't identical with gnu-gas in every case (this code produces
fewer identical FREs in a row than gas), but I'm reasonably sure that
they are correct regardless. There are even more size optimizations that
can be done later.
Also, while working on the tests, I found a few bugs in older portions
and cleaned those up.
This is a fairly big commit, but I'm not sure how to make it smaller.